Skip to content

A binding reaches where Jinja says it reaches, and no further - #461

Merged
jeremymanning merged 3 commits into
mainfrom
fix/scope-aware-references
Aug 3, 2026
Merged

A binding reaches where Jinja says it reaches, and no further#461
jeremymanning merged 3 commits into
mainfrom
fix/scope-aware-references

Conversation

@jeremymanning

Copy link
Copy Markdown
Member

Fixes the false negative in reference extraction reported against #458.

The defect

Both validators answered "is this name the template's own?" with one template-wide set of every bound name. That is right for a name used only where it is bound, and wrong everywhere else — binding a name anywhere silenced it everywhere:

{{ ghost.done }}                <- undefined; reported nothing
{% for ghost in rows %}
  {{ ghost.name }}              <- the binding that silenced it
{% endfor %}

The reference outside the loop is exactly the typo a validator exists to catch, and adding an unrelated loop elsewhere in the file made it vanish. The trade was backwards: the false positive this replaced was loud, a false negative is silent.

The fix

core/template_scope.py tracks scope the way Jinja does. A binding reaches the body of the construct that introduces it and nothing outside it; {% set %} reaches the statements that follow it in the same block. {% if %} opens no scope, because Jinja gives it none — treating its {% set %} as reaching the rest of the block can only suppress a report, never invent one.

Identity, not spelling: shadowed_name_nodes returns the id() of each Name node that refers to a local binding, so one template can hold both a shadowed and an unshadowed use of the same word.

Both validators ask it, so they cannot disagree about what a template defines.

All seven cases from the review are covered, plus: the iterable evaluated outside its own loop, {% for %}'s filter test, the {% else %} branch, loop itself, tuple targets, {% set %} reading its own prior value, {% call %} arguments, and macro defaults.

Verification

  • Reverting to the template-wide bound set fails 20 tests.
  • Blocking suite: 772 passed, 13 skipped, 0 failed.
  • Catalogue: 50/117, unchanged — the fix reports strictly more, and no validating example depended on the over-broad suppression.
  • Lint clean.

Two things worth flagging

The end-to-end guard initially passed under the very mutation it existed to catch. Two reasons: the probe pipeline's rows is undeclared, so it is refused either way; and the template validator finds undeclared names with jinja2.meta, which is scope-aware already and reported ghost on its own. Only the data-flow validator was blind. The assertion now targets its specific phrasing, and fails under the mutation as it should.

Three dead imports removedjinja2.meta, TemplateSyntaxError and Union in data_flow_validator.py. meta reports only top-level undeclared names, which is why the chain and subscript forms had to be recovered from raw text; walking the AST replaced both, and these have had no caller since #458.

🤖 Generated with Claude Code

jeremymanning and others added 3 commits August 3, 2026 19:36
Both validators answered "is this name the template's own?" with one
template-wide set of every bound name. That is right for a name used only
where it is bound and wrong everywhere else, because binding a name anywhere
silenced it everywhere:

    {{ ghost.done }}            <- undefined; reported nothing
    {% for ghost in rows %}
      {{ ghost.name }}          <- the binding that silenced it
    {% endfor %}

The reference outside the loop is exactly the typo a validator exists to
catch, and adding an unrelated loop to the file made it vanish. The trade was
backwards: the false positive this replaced was loud, a false negative is
silent.

core/template_scope.py tracks scope as Jinja does. A binding reaches the body
of the construct that introduces it and nothing outside it; {% set %} reaches
the statements that follow it in the same block. {% if %} opens no scope,
because Jinja gives it none.

Identity, not spelling: shadowed_name_nodes returns the id() of each Name
node that refers to a local binding, so one template can hold both a shadowed
and an unshadowed use of the same word.

Both validators ask it, so they cannot disagree about what a template defines.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`jinja2.meta` was how references used to be found. It reports only
top-level undeclared names, which is why the chain and subscript forms had
to be recovered from the raw text afterwards -- the text-chopping #458
removed. Walking the AST replaced both, and these three imports have had no
caller since.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The first version of this guard passed under the very mutation it existed to
catch. Asserting that validation fails proves nothing here: `rows` is
undeclared, so the pipeline is refused either way, and the template validator
finds undeclared names with jinja2.meta -- which is scope-aware already and
reports 'ghost' on its own.

The data-flow validator was the blind one. It has its own phrasing, so that
is what the assertion looks for.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant